Skip to content

feat(skills): add cli skill test command#3999

Open
JingHou1215 wants to merge 1 commit intoagentscope-ai:mainfrom
JingHou1215:feature/skills-test-cli
Open

feat(skills): add cli skill test command#3999
JingHou1215 wants to merge 1 commit intoagentscope-ai:mainfrom
JingHou1215:feature/skills-test-cli

Conversation

@JingHou1215
Copy link
Copy Markdown

@JingHou1215 JingHou1215 commented May 2, 2026 •
edited
Loading

Description

Adds an initial CLI entrypoint for validating individual QwenPaw skills before assigning them to agents.

This PR introduces qwenpaw skills test <skill>, supporting validation by workspace skill name or local skill directory path. The command checks that SKILL.md exists, validates required frontmatter fields (name and description), and runs the existing skill security scanner with blocking enabled.

Related Issue: Relates to #3883

Security Considerations: Uses the existing skill security scanner, so skills with blocking security findings fail validation.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Refactoring

Component(s) Affected

  • Core / Backend (app, agents, config, providers, utils, local_models)
  • Console (frontend web UI)
  • Channels (DingTalk, Feishu, QQ, Discord, iMessage, etc.)
  • Skills
  • CLI
  • Documentation (website)
  • Tests
  • CI/CD
  • Scripts / Deploy

Checklist

  • I ran pre-commit run --all-files locally and it passes
  • If pre-commit auto-fixed files, I committed those changes and reran checks
  • I ran tests locally (pytest or as relevant) and they pass
  • Documentation updated (if needed)
  • Ready for review

For Channel Changes (DingTalk, Feishu, QQ, Console, etc.)

  • I ran ./scripts/check-channels.sh (or ./scripts/check-channels.sh --changed) and it passes
  • Contract test exists in tests/contract/channels/test_<channel>_contract.py (REQUIRED)
  • Contract test implements create_instance() with proper channel initialization
  • All 19 contract verification points pass (see tests/contract/channels/__init__.py)
  • Optional: Unit tests in tests/unit/channels/test_<channel>.py for complex internal logic

Testing

  • .\.venv\Scripts\pre-commit.exe run --files src\qwenpaw\cli\skills_cmd.py → passed

Local Verification Evidence

  • pre-commit run --all-files → passed
  • pytest → passed

@github-project-automation github-project-automation Bot moved this to Todo in QwenPaw May 2, 2026
@github-actions github-actions Bot added the first-time-contributor PR created by a first time contributor label May 2, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 2, 2026

Welcome to QwenPaw! 🐾

Hi @JingHou1215, thank you for your first Pull Request! 🎉

🙌 Join Developer Community

Thanks so much for your contribution! We'd love to invite you to join the official QwenPaw developer group! You can find the Discord and DingTalk group links under the "Developer Community" section on our docs page:
https://qwenpaw.agentscope.io/docs/community

We truly appreciate your enthusiasm—and look forward to your future contributions! 😊

We'll review your PR soon.

This was referenced May 2, 2026
Open
Open
Open
Open
@xieyxclack xieyxclack requested a review from Leirunlin May 3, 2026 03:11
@Leirunlin
Copy link
Copy Markdown
Collaborator

Leirunlin commented May 6, 2026 •
edited
Loading

Hi! @JingHou1215 Thanks for the PR. I have reviewed the PR and have the below suggestions:

  • For workspace skills, runtime behavior is still keyed by directory name, so skills test should not switch to using the declared frontmatter name for scan identity / output. If those two drift, this command can validate or report the skill under a different name than the rest of the system uses, which is misleading and can affect scan/whitelist behavior.
    I think this should keep directory name as the primary identity for workspace skills, and use frontmatter only for validation.

  • No additional unit test is needed here.

@JingHou1215 JingHou1215 force-pushed the feature/skills-test-cli branch 2 times, most recently from cbfa8ca to d4bef86 Compare May 7, 2026 02:47
@JingHou1215
Copy link
Copy Markdown
Author

JingHou1215 commented May 7, 2026

Thanks for the review, @Leirunlin!

Updated. Workspace skill test now uses the directory name as the scan/output identity, while frontmatter is only used for validation. I also removed the added unit test file per your feedback.

@Leirunlin
Copy link
Copy Markdown
Collaborator

Leirunlin commented May 7, 2026

@JingHou1215 Hi! This looks better now.

A small point: skills test currently parses frontmatter manually in src/qwenpaw/cli/skills_cmd.py (_parse_skill_frontmatter), while the existing project logic uses frontmatter.loads(...) in src/qwenpaw/agents/ skills_manager.py (_validate_skill_content / _read_frontmatter).

The manual parsers only detects closing fence with line.strip() in {"---", "..."}. How about re-use the implementation of current project? It could be cleaner.

@JingHou1215 JingHou1215 force-pushed the feature/skills-test-cli branch from d4bef86 to 010e778 Compare May 7, 2026 09:04
@JingHou1215
Copy link
Copy Markdown
Author

JingHou1215 commented May 7, 2026

Thanks for the detailed and patient feedback! @Leirunlin

Updated as suggested. skills test now reuses _validate_skill_content(content) from skills_manager.py, which uses frontmatter.loads(...) internally. I also removed the manual parser and unused imports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Leirunlin Leirunlin Awaiting requested review from Leirunlin

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

first-time-contributor PR created by a first time contributor Under Review

Projects

QwenPaw
Status: Todo

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JingHou1215 @Leirunlin